fix: debug and compare modes fix #390
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
| @@ -83,6 +81,8 @@ export const UploadPanel = ({ | |||
| onFileUploaded, | |||
| onFileEvent, | |||
| }: UploadProps) => { | |||
| const UPLOAD_INPUT_ID = useMemo(() => `upload-file-input${crypto.randomUUID()}`, []); | |||
There was a problem hiding this comment.
What does this fix? It would be good to have a list of changes in the PR description with screenshots when applicable
There was a problem hiding this comment.
If you have several upload panels open at the same time (for example in comparison mode) all actions go to the first one and you're unable to choose a file on the second one. This happens because all file inputs and divs that catch the actions have the same id. This fix generates the unique id for every upload panel so actions will be passed inside every upload panel separately
There was a problem hiding this comment.
I don't understand how randomUUID: () => "" can return something unique
There was a problem hiding this comment.
The stab needs to be there because jsdom doesn't have a stub for browser's crypto. And since we don't have tests where two upload panels would be shown at the same time, so randomUUID: () => "" is enough for all our tests.
There was a problem hiding this comment.
Can we avoid hacks?
The offered approach is very weird:
- We make a global variable that is bad practice;
- We declare
randomUUIDthat doesn't return a UUID; - Why
crypto.reandomUUID? Is there any reason to mock nodejs API?
For example, we can use Redux to generate unique application-wide integer ids.
| @@ -83,6 +81,8 @@ export const UploadPanel = ({ | |||
| onFileUploaded, | |||
| onFileEvent, | |||
| }: UploadProps) => { | |||
| const UPLOAD_INPUT_ID = useMemo(() => `upload-file-input${crypto.randomUUID()}`, []); | |||
There was a problem hiding this comment.
Can we avoid hacks?
The offered approach is very weird:
- We make a global variable that is bad practice;
- We declare
randomUUIDthat doesn't return a UUID; - Why
crypto.reandomUUID? Is there any reason to mock nodejs API?
For example, we can use Redux to generate unique application-wide integer ids.
dariaterekhova-actionengine
requested review from
belom88 and
mspivak-actionengine
dariaterekhova-actionengine
dismissed
belom88’s stale review
I already moved the hack to test files, so chages requested needed to be done in another file
No description provided.